fix: preserve unqualified references in methods - #10679
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
You’re at about 99% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
df71755 to
68a78f3
Compare
There was a problem hiding this comment.
Pull request overview
This PR refines ScopedVisitor’s name-resolution model by generalizing scope tracking (beyond comprehensions) and treating class-defined functions as “methods” so unqualified references inside method bodies are preserved as external refs (matching Python’s runtime lookup rules).
Changes:
- Introduces a
LexicalScopeenum and replaces the prioris_comprehensionflag with a generalizedscope_kind. - Distinguishes
VariableData.kind="method"vs"function"forFunctionDef/AsyncFunctionDefbased on whether the enclosing scope is a class. - Adds a regression test covering the difference between method bodies (don’t resolve through class scope) and nested functions (do resolve through enclosing function scope).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
marimo/_ast/visitor.py |
Generalizes scope tracking and adjusts reference/definition handling to preserve unqualified refs in methods. |
tests/_ast/test_visitor.py |
Adds a regression test for method-vs-function name resolution behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| def test_function_name_resolution_in_class_and_function_scopes() -> None: | ||
| code = cleandoc( | ||
| """ | ||
| class C: | ||
| def method(self): | ||
| return method() | ||
|
|
||
| def outer(): | ||
| def method(): | ||
| return method() | ||
| return method | ||
| """ | ||
| ) | ||
| v = visitor.ScopedVisitor() | ||
| mod = ast.parse(code) | ||
| v.visit(mod) | ||
|
|
||
| # A bare name in a method body does not resolve through the class scope. | ||
| assert v.refs == {"method"} | ||
| assert v.variable_data["C"][0].required_refs == {"method"} | ||
|
|
||
| # A nested function does resolve its name through its enclosing function. | ||
| assert v.variable_data["outer"][0].required_refs == set() | ||
|
|
||
|
|
Summary
Followup for the comments on #10678.
We previously only had a scope distinction for comprehensions (
is_comprehension), the review bots in #10678, rightly caught that because class scoping works differently from function scoping, the recursive reference calls did not work in classes.This PR: